Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DID rotation signature to 23 #795

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

TelegramSam
Copy link
Contributor

Fixes #717

Signed-off-by: Sam Curren <telegramsam@gmail.com>
@@ -325,6 +325,19 @@ The exchange response message is used to complete the exchange. This message is
"signature": "3dZWsuru7QAVFUCtTd0s7uc1peYEijx4eyt5... (bytes omitted)"
}
}
},
"did_rotate~attach": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two questions to consider — more to know why you did it this way then to propose changes.

  • Should this be a change to the protocol version to 1.1?
    • It is a new item, so I wanted to check to see if you think there should be a version bump.
  • Why the “_” and “-“ in the same item?
    • I see it in the did_doc-attach, so assume that is the reason, but thought I would ask :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second item: it's a tilde, not a dash.
first item - maybe/probably? A topic for tomorrow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! Sorry about that — it flashed through my head that the attach was a decorator, but didn’t stick, and not when I was looking at the item name :-).

That makes it easier for this to be a no version change. The guidance is to put a decorator on it with this content vs. the must. Of course, any implementation can do what they think is appropriate — either including the ~attach decorator or not, ignoring or processing the decorator on receipt, and rejecting or not the connection if NOT included.

Comment on lines +355 to 356
* The `did_rotate~attach` attribute is optional, but SHOULD be included if the `did` attribute is resolvable and the `did_doc~attach` is not included. The value is the Base64url encoded DID, and signed with the key used in the invitation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should include that if a did is used in the response message that is different from the invitation, the did_rotate~attach MUST be present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great wording, though this implies a stronger MUST requirement than we usually allow for a minor version change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MUST is in the eye of the recipient. If they choose to reject a connection because its not included, then so be it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need more language - feels wrong (warren) to not have a MUST included.

Copy link
Contributor

@genaris genaris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the request message? Is it some clarification needed in regards of the signature for the DID Doc attachment?

From #717:

Signatures were never needed in the Requests, and were added by mistake by... me. Does this address your confusion? (@TelegramSam @ #717 (comment))

features/0023-did-exchange/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe is it a good opportunity to remove the double of @swcurran from this list? Not sure if it is the first one or the second one 😝

Co-authored-by: Ariel Gentile <gentilester@gmail.com>
Signed-off-by: Sam Curren <telegramsam@gmail.com>
@TelegramSam
Copy link
Contributor Author

What about the request message? Is it some clarification needed in regards of the signature for the DID Doc attachment?

The signature in the response is used to prove ownership over the key used in the invitation. In the request message, the only key they could prove ownership of is the one for the DID presented in the message, and that is already proven by sending an authcrypted message, per standard practice.

Am I correct here?

@TelegramSam
Copy link
Contributor Author

Discussed WG 20231004. Needs to bump the version to 1.1

Signed-off-by: Sam Curren <telegramsam@gmail.com>
Copy link
Member

@swcurran swcurran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed and agreed to be merged at Aries Working Group Call 2023.12.06.

@swcurran swcurran merged commit a3ce439 into hyperledger:main Dec 6, 2023
1 check passed
@TelegramSam TelegramSam deleted the 23_optional_signed_did branch December 12, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC 0023: Can we accept inline peer did without a signature?
4 participants